remove specific model checks for general scitype check #731
Conversation
| "using the syntax `machine(model, X, y)` or `machine(model, X, y, w)` " * | ||
| "while most other models with `machine(model, X)`. " * | ||
| "Here `X` are features, `y` a target, and `w` sample or class weights. " * | ||
| "In general, data in `machine(model, data...)` must satisfy " * |
There was a problem hiding this comment.
Line 102: Put "data" in back-quotes: "`data`".
| "`scitype(data) <: MLJ.fit_data_scitype(model)` unless the " * | ||
| "right-hand side is `Unknown`. Here, the scitype of `args` " * | ||
| "in `machine(model, args...; kwargs)` does not match the scitype " * | ||
| "expected by model's `fit` method.\n" * | ||
| " provided: $S\n expected by fit: $F" |
There was a problem hiding this comment.
How about we replace the last sentence (from the original string, which is using different notation) with:
"In the present case:\n"*
"scitype(data) = $S\n"*
"and\n"*
"fit_data_scitype(model) = $F"
| "while most other models with `machine(model, X)`. " * | ||
| "Here `X` are features, `y` a target, and `w` sample or class weights. " * | ||
| "In general, data in `machine(model, data...)` must satisfy " * | ||
| "`scitype(data) <: MLJ.fit_data_scitype(model)` unless the " * |
There was a problem hiding this comment.
We can drop the qualifier MLJ. from MLJ.fit_data_scitype(model) as this is now exported.
|
@pazzo83 Regarding the failing log tests. Rather than something like the existing (:warn, r"The scitype of `X`")let's make this robust to changes to the doc-string by instead doing something like (:warn, MLJBase.warn_generic_scitype_mismatch(S, F))where Make sense? |
|
Okay, after looking again at the code, JuliaAI/ScientificTypes.jl#182 is not an issue. I thought we were doing Line 135 in 3b85789 So, I think your fail is what I first suggested: outdated logging tests. The error message thrown has changed, so we have to update the |
|
Hang on. I think |
|
Yeah I had originally changed it to vs |
|
@pazzo83 I'll have a proper look into this tomorrow, thanks. |
|
@pazzo83 Looking into this now and making progress. Thanks for your continued patience. |
|
Closing in favour of #732 |
First pass at simplifying type checks for machines - we simply check the expected scitype signature instead of specific checks for supervised, unsupervised, etc.